fix(acp): require closing old session before switching - #396

Security Blast Radius

• inferred — The new admission control limits repeated setup through one connection before another session environment can be assembled. It does not establish a process-wide resource quota or a tenant-isolation boundary, and no broader containment guarantee was demonstrated.

Trust Boundaries and Controls

• observed — Caller-supplied setup methods and session IDs reach a shared connection reservation before environment assembly. Generation checks prevent stale reservations from overwriting newer connection state; successful close or delete releases only the matching current session.

Resilience and Maintainability Implications

• observed — Internal recovery launch reuses an existing runtime and checks its generation, closing state, active turn, and activation permission before scheduling continuation. Host shutdown separately retires connection endpoints and drains owned session resources, providing counterevidence against treating shutdown-time load cancellation as a persistent live-connection bypass.

Explanation

The title "fix(acp): require closing old session before switching" matches the main change in the changeset. The raw summary and PR objectives show a connection-scoped session-switch gate. Setup for a different session is rejected until the bound session is successfully closed or deleted. The same-ID load/resume and read-only history paths remain allowed, and the title covers that core rule. The title is concise and specific, and it follows a conventional commit style. It does not mention cleanup ownership or recovery changes, but that detail is expected to be omitted.

Explanation

Issue #394 is the only active direct link. Its coding requirements, with evidence from the reviewed PR summaries:

1. [#394] Reject session/new and session/fork when a session is current. Reject different-ID session/load and session/resume when a session is current. Allow same-ID reload. peri-acp/src/host/connection.rs adds reserve_session_setup with -32010 and reason session_switch_requires_close. Connection tests and requests_close_failure_test.rs cover this.
2. [#394] Linearize setup through a connection-level reservation. SessionSetupReservation is generation-guarded. Concurrent setup returns session_setup_in_progress. connection_test.rs covers rejection of concurrent setup.
3. [#394] Bind the session ID only after the setup response is sent. Release it only after a matching close or delete response is sent. server_loop.rs calls commit after the response is sent and calls release_current_session only for a successful close or delete. history_page_wire_test.rs covers a failed response write.
4. [#394] Read-only cursor, window, body, and child-history requests bypass the gate. Internal recovery children bypass the gate. history_page_wire_test.rs adds bypass tests. The recovery child path is reported as covered by the recovery_child tests.
5. [#394] Skip list_session_tree when no working child exists. recovery/coordinator.rs queries the tree only when children exist. recovery_wire_test.rs adds recovery_wire_root_only_load_skips_session_tree_query.
6. [#394] Failed setup must not leave an unbound live environment. load_shared.rs and fork_shared.rs retain the request-owned publication and roll back only the matching generation. Tests in history_page_initial_cursor_test.rs and history_page_window_lock_test.rs cover this.
7. [#394] Cleanup-incomplete keeps the owner bound until an explicit close succeeds. server_loop.rs binds the reservation for session_cleanup_incomplete with a nonempty sessionId. This blocks other session setup until close succeeds.
8. [#394] Schema, index, automatic LRU/close, shared MCP pool, and post-OOM recovery are excluded. The summaries show none of these changes.

The TUI client change (peri-tui transition.rs and recovery_test.rs) remembers the cleanup-incomplete owner and closes it before the next switch. This matches the approved contract that the client closes the current session before switching. It does not conflict with #394.

Limits: I did not read the full whole-PR diff. I did not run the reported tests. The pass result relies on the PR summaries, the previous assessment, and the reported validation. The real Hindsight 512MB cgroup check is deployment validation, not a coding requirement.

No unmet coding requirement is established.

Explanation

The changed areas map to #394 objectives or to direct supporting work:

• connection.rs, connection_test.rs, server_loop.rs: session-switch state machine, setup gate, commit and release, and cleanup-incomplete binding.
• server_loop/notification.rs: notification dispatch moved out of server_loop.rs to keep it under the 1000-line limit. The PR describes this as a refactor. It does not change behavior that the issue requires.
• load_shared.rs, fork_shared.rs, session_restore.rs, session_lifecycle.rs: request-owned cold-restore publication rollback and fork source cleanup. These support the failed-setup criterion.
• recovery/coordinator.rs, recovery_wire_test.rs: skip of the session-tree query. This is an #394 criterion.
• History, close-failure, endpoint, and initial-cursor tests: regression coverage for the gate, same-ID reuse, read-only bypass, close failure, and fork source handling.
• peri-tui transition.rs and recovery_test.rs: client-side memory of the cleanup owner, so the next switch closes it first. This supports the close-before-switch contract in #394.
• Docs, architecture contract, code index, and spec/issues record: documentation of the new contract.

No change is found that is unrelated to #394. Limits: I did not read the full diff, so this rests on the summaries.

• Commit to this branch
• Create a new PR